Skip to content

Conversation

barsolo2000
Copy link
Contributor

@barsolo2000 barsolo2000 commented Sep 11, 2025

Added emulator unwinding support for RISCV files.

Emulated Instructions:
ADD (addi sp, sp, imm)
STORE (sd ra, offset(sp))
LOAD (ld ra, offset(sp)).

We had to overwrite SetInstructions() since UnwindAssemblyInstEmulation calls EvaluateInstruction() directly after calling SetInstruction(), but it never calls ReadInstruction(). This means that the m_decoded member variable in the instruction emulator is never properly initialized. By overriding SetInstruction(), we decode the instruction bytes and set m_decoded directly. This ensures that subsequent emulation (including unwinding) operates on the correct instruction.

We also had to change the the OpCode GetOpcodeBytes function since recent changes made it so GetOpcodeBytes will return None for type eType16_32Tuples (an alternative and longer way, would've been to type check during the overwritten SetInstruction() and call a DataExtractor with .GetU16(&offset) to set the inst_data.

Added a test - TestSimpleRiscvFunction (took inspiration from: link)

[----------] 1 test from TestRiscvInstEmulation
[ RUN ] TestRiscvInstEmulation.TestSimpleRiscvFunction
[ OK ] TestRiscvInstEmulation.TestSimpleRiscvFunction (1 ms)
[----------] 1 test from TestRiscvInstEmulation (1 ms total)

[----------] Global test environment tear-down
[==========] 63 tests from 5 test suites ran. (11 ms total)
[ PASSED ] 63 tests.

@satyajanga
Copy link
Contributor

update the description with more details.

  1. which instructions are emulated.
  2. why the new members are overrided.
  3. why the OpCode change needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this unwind plan truly valid at all instructions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from my understanding, No, this unwind plan is NOT valid at all instructions because it assumes SP+0 and unchanged registers which becomes incorrect after prologue instructions execute. eLazyBoolYes means valid at every instruction while eLazyBoolNo means only valid at call sites/exception points, so this basic function entry plan should use eLazyBoolNo. It seems like other emulators (ARM, MIPS for example) are using the same things.

Copy link

github-actions bot commented Sep 12, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@barsolo2000 barsolo2000 marked this pull request as ready for review September 12, 2025 20:43
@llvmbot
Copy link
Member

llvmbot commented Sep 12, 2025

@llvm/pr-subscribers-backend-risc-v

@llvm/pr-subscribers-lldb

Author: None (barsolo2000)

Changes

Added emulator unwinding support for RISCV files.

Emulated Instructions:
ADD (addi sp, sp, imm)
STORE (sd ra, offset(sp))
LOAD (ld ra, offset(sp)).

We had to overwrite SetInstructions() since UnwindAssemblyInstEmulation calls EvaluateInstruction() directly after calling SetInstruction(), but it never calls ReadInstruction(). This means that the m_decoded member variable in the instruction emulator is never properly initialized. By overriding SetInstruction(), we decode the instruction bytes and set m_decoded directly. This ensures that subsequent emulation (including unwinding) operates on the correct instruction.

We also had to change the the OpCode GetOpcodeBytes function since recent changes made it so GetOpcodeBytes will return None for type eType16_32Tuples (an alternative and longer way, would've been to type check during the overwritten SetInstruction() and call a DataExtractor with .GetU16(&offset) to set the inst_data.

Added a test - TestSimpleRiscvFunction (took inspiration from: link)

[----------] 1 test from TestRiscvInstEmulation
[ RUN ] TestRiscvInstEmulation.TestSimpleRiscvFunction
[ OK ] TestRiscvInstEmulation.TestSimpleRiscvFunction (1 ms)
[----------] 1 test from TestRiscvInstEmulation (1 ms total)

[----------] Global test environment tear-down
[==========] 63 tests from 5 test suites ran. (11 ms total)
[ PASSED ] 63 tests.


Full diff: https://github.com/llvm/llvm-project/pull/158161.diff

5 Files Affected:

  • (modified) lldb/include/lldb/Core/Opcode.h (+3-1)
  • (modified) lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp (+162-12)
  • (modified) lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h (+4)
  • (modified) lldb/unittests/Instruction/CMakeLists.txt (+5)
  • (added) lldb/unittests/Instruction/RISCV/TestRiscvInstEmulation.cpp (+188)
diff --git a/lldb/include/lldb/Core/Opcode.h b/lldb/include/lldb/Core/Opcode.h
index 7bbd73d039f99..7e756d3f15d22 100644
--- a/lldb/include/lldb/Core/Opcode.h
+++ b/lldb/include/lldb/Core/Opcode.h
@@ -223,7 +223,9 @@ class Opcode {
   int Dump(Stream *s, uint32_t min_byte_width) const;
 
   const void *GetOpcodeBytes() const {
-    return ((m_type == Opcode::eTypeBytes) ? m_data.inst.bytes : nullptr);
+    return ((m_type == Opcode::eTypeBytes || m_type == Opcode::eType16_32Tuples)
+                ? m_data.inst.bytes
+                : nullptr);
   }
 
   uint32_t GetByteSize() const {
diff --git a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
index 5e429a92613ce..36e6bd41ad7fa 100644
--- a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
+++ b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
@@ -33,6 +33,10 @@ LLDB_PLUGIN_DEFINE_ADV(EmulateInstructionRISCV, InstructionRISCV)
 
 namespace lldb_private {
 
+// RISC-V General Purpose Register numbers
+static constexpr uint32_t RISCV_GPR_SP = 2; // x2 is the stack pointer
+static constexpr uint32_t RISCV_GPR_FP = 8; // x8 is the frame pointer
+
 /// Returns all values wrapped in Optional, or std::nullopt if any of the values
 /// is std::nullopt.
 template <typename... Ts>
@@ -108,6 +112,14 @@ static uint32_t FPREncodingToLLDB(uint32_t reg_encode) {
   return LLDB_INVALID_REGNUM;
 }
 
+// Helper function to get register info from GPR encoding
+static std::optional<RegisterInfo>
+GPREncodingToRegisterInfo(EmulateInstructionRISCV &emulator,
+                          uint32_t reg_encode) {
+  uint32_t lldb_reg = GPREncodingToLLDB(reg_encode);
+  return emulator.GetRegisterInfo(eRegisterKindLLDB, lldb_reg);
+}
+
 bool Rd::Write(EmulateInstructionRISCV &emulator, uint64_t value) {
   uint32_t lldb_reg = GPREncodingToLLDB(rd);
   EmulateInstruction::Context ctx;
@@ -230,10 +242,34 @@ Load(EmulateInstructionRISCV &emulator, I inst, uint64_t (*extend)(E)) {
   auto addr = LoadStoreAddr(emulator, inst);
   if (!addr)
     return false;
-  return transformOptional(
-             emulator.ReadMem<T>(*addr),
-             [&](T t) { return inst.rd.Write(emulator, extend(E(t))); })
-      .value_or(false);
+
+  // Set up context for the load operation, similar to ARM64
+  EmulateInstructionRISCV::Context context;
+
+  // Get register info for base register
+  std::optional<RegisterInfo> reg_info_rs1 =
+      GPREncodingToRegisterInfo(emulator, inst.rs1.rs);
+
+  if (!reg_info_rs1)
+    return false;
+
+  // Set context type based on whether this is a stack-based load
+  if (inst.rs1.rs == RISCV_GPR_SP) // x2 is the stack pointer in RISC-V
+    context.type = EmulateInstruction::eContextPopRegisterOffStack;
+  else
+    context.type = EmulateInstruction::eContextRegisterLoad;
+
+  // Set the context address information
+  context.SetAddress(*addr);
+
+  // Read from memory with context and write to register
+  bool success = false;
+  uint64_t value =
+      emulator.ReadMemoryUnsigned(context, *addr, sizeof(T), 0, &success);
+  if (!success)
+    return false;
+
+  return inst.rd.Write(emulator, extend(E(T(value))));
 }
 
 template <typename I, typename T>
@@ -242,9 +278,35 @@ Store(EmulateInstructionRISCV &emulator, I inst) {
   auto addr = LoadStoreAddr(emulator, inst);
   if (!addr)
     return false;
-  return transformOptional(
-             inst.rs2.Read(emulator),
-             [&](uint64_t rs2) { return emulator.WriteMem<T>(*addr, rs2); })
+
+  // Set up context for the store operation, similar to ARM64
+  EmulateInstructionRISCV::Context context;
+
+  // Get register info for source and base registers
+  std::optional<RegisterInfo> reg_info_rs1 =
+      GPREncodingToRegisterInfo(emulator, inst.rs1.rs);
+  std::optional<RegisterInfo> reg_info_rs2 =
+      GPREncodingToRegisterInfo(emulator, inst.rs2.rs);
+
+  if (!reg_info_rs1 || !reg_info_rs2)
+    return false;
+
+  // Set context type based on whether this is a stack-based store
+  if (inst.rs1.rs == RISCV_GPR_SP) // x2 is the stack pointer in RISC-V
+    context.type = EmulateInstruction::eContextPushRegisterOnStack;
+  else
+    context.type = EmulateInstruction::eContextRegisterStore;
+
+  // Set the context to show which register is being stored to which base
+  // register + offset
+  context.SetRegisterToRegisterPlusOffset(*reg_info_rs2, *reg_info_rs1,
+                                          SignExt(inst.imm));
+
+  return transformOptional(inst.rs2.Read(emulator),
+                           [&](uint64_t rs2) {
+                             return emulator.WriteMemoryUnsigned(
+                                 context, *addr, rs2, sizeof(T));
+                           })
       .value_or(false);
 }
 
@@ -737,11 +799,44 @@ class Executor {
   bool operator()(SH inst) { return Store<SH, uint16_t>(m_emu, inst); }
   bool operator()(SW inst) { return Store<SW, uint32_t>(m_emu, inst); }
   bool operator()(ADDI inst) {
-    return transformOptional(inst.rs1.ReadI64(m_emu),
-                             [&](int64_t rs1) {
-                               return inst.rd.Write(
-                                   m_emu, rs1 + int64_t(SignExt(inst.imm)));
-                             })
+    return transformOptional(
+               inst.rs1.ReadI64(m_emu),
+               [&](int64_t rs1) {
+                 int64_t result = rs1 + int64_t(SignExt(inst.imm));
+                 // Check if this is a stack pointer adjustment
+                 if (inst.rd.rd == RISCV_GPR_SP &&
+                     inst.rs1.rs == RISCV_GPR_SP) { // rd=sp, rs1=sp
+                   EmulateInstruction::Context context;
+                   context.type =
+                       EmulateInstruction::eContextAdjustStackPointer;
+                   context.SetImmediateSigned(SignExt(inst.imm));
+                   uint32_t sp_lldb_reg = GPREncodingToLLDB(RISCV_GPR_SP);
+                   RegisterValue registerValue;
+                   registerValue.SetUInt64(result);
+                   return m_emu.WriteRegister(context, eRegisterKindLLDB,
+                                              sp_lldb_reg, registerValue);
+                 }
+                 // Check if this is setting up the frame pointer
+                 // addi fp, sp, imm -> fp = sp + imm (frame pointer setup)
+                 if (inst.rd.rd == RISCV_GPR_FP &&
+                     inst.rs1.rs == RISCV_GPR_SP) { // rd=fp, rs1=sp
+                   EmulateInstruction::Context context;
+                   context.type = EmulateInstruction::eContextSetFramePointer;
+                   auto sp_reg_info = m_emu.GetRegisterInfo(
+                       eRegisterKindLLDB, GPREncodingToLLDB(RISCV_GPR_SP));
+                   if (sp_reg_info) {
+                     context.SetRegisterPlusOffset(*sp_reg_info,
+                                                   SignExt(inst.imm));
+                   }
+                   uint32_t fp_lldb_reg = GPREncodingToLLDB(RISCV_GPR_FP);
+                   RegisterValue registerValue;
+                   registerValue.SetUInt64(result);
+                   return m_emu.WriteRegister(context, eRegisterKindLLDB,
+                                              fp_lldb_reg, registerValue);
+                 }
+                 // Regular ADDI instruction
+                 return inst.rd.Write(m_emu, result);
+               })
         .value_or(false);
   }
   bool operator()(SLTI inst) {
@@ -1745,6 +1840,61 @@ EmulateInstructionRISCV::GetRegisterInfo(RegisterKind reg_kind,
   return array[reg_index];
 }
 
+bool EmulateInstructionRISCV::SetInstruction(const Opcode &opcode,
+                                             const Address &inst_addr,
+                                             Target *target) {
+  // Call the base class implementation
+  if (!EmulateInstruction::SetInstruction(opcode, inst_addr, target))
+    return false;
+
+  // Extract instruction data from the opcode
+  uint32_t inst_data = 0;
+  const void *opcode_data = m_opcode.GetOpcodeBytes();
+  if (!opcode_data)
+    return false;
+
+  if (m_opcode.GetByteSize() == 2) {
+    // 16-bit compressed instruction
+    const uint16_t *data = static_cast<const uint16_t *>(opcode_data);
+    inst_data = *data;
+  } else if (m_opcode.GetByteSize() == 4) {
+    // 32-bit instruction
+    const uint32_t *data = static_cast<const uint32_t *>(opcode_data);
+    inst_data = *data;
+  } else {
+    return false;
+  }
+
+  // Decode the instruction
+  auto decoded_inst = Decode(inst_data);
+  if (!decoded_inst)
+    return false;
+
+  // Store the decoded result
+  m_decoded = *decoded_inst;
+  return true;
+}
+
+bool EmulateInstructionRISCV::CreateFunctionEntryUnwind(
+    UnwindPlan &unwind_plan) {
+  unwind_plan.Clear();
+  unwind_plan.SetRegisterKind(eRegisterKindLLDB);
+
+  UnwindPlan::Row row;
+
+  row.GetCFAValue().SetIsRegisterPlusOffset(gpr_sp_riscv, 0);
+  row.SetRegisterLocationToSame(gpr_ra_riscv, /*must_replace=*/false);
+  row.SetRegisterLocationToSame(gpr_fp_riscv, /*must_replace=*/false);
+
+  unwind_plan.AppendRow(std::move(row));
+  unwind_plan.SetSourceName("EmulateInstructionRISCV");
+  unwind_plan.SetSourcedFromCompiler(eLazyBoolNo);
+  unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolYes);
+  unwind_plan.SetUnwindPlanForSignalTrap(eLazyBoolNo);
+  unwind_plan.SetReturnAddressRegister(gpr_ra_riscv);
+  return true;
+}
+
 bool EmulateInstructionRISCV::SetTargetTriple(const ArchSpec &arch) {
   return SupportsThisArch(arch);
 }
diff --git a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
index 3578a4ab03053..c196a9bb9ce82 100644
--- a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
+++ b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
@@ -61,6 +61,7 @@ class EmulateInstructionRISCV : public EmulateInstruction {
     case eInstructionTypePCModifying:
       return true;
     case eInstructionTypePrologueEpilogue:
+      return true;
     case eInstructionTypeAll:
       return false;
     }
@@ -85,6 +86,7 @@ class EmulateInstructionRISCV : public EmulateInstruction {
     return SupportsThisInstructionType(inst_type);
   }
 
+  bool CreateFunctionEntryUnwind(UnwindPlan &unwind_plan) override;
   bool SetTargetTriple(const ArchSpec &arch) override;
   bool ReadInstruction() override;
   std::optional<uint32_t> GetLastInstrSize() override { return m_last_size; }
@@ -94,6 +96,8 @@ class EmulateInstructionRISCV : public EmulateInstruction {
   std::optional<RegisterInfo> GetRegisterInfo(lldb::RegisterKind reg_kind,
                                               uint32_t reg_num) override;
 
+  bool SetInstruction(const Opcode &opcode, const Address &inst_addr,
+                      Target *target) override;
   std::optional<DecodeResult> ReadInstructionAt(lldb::addr_t addr);
   std::optional<DecodeResult> Decode(uint32_t inst);
   bool Execute(DecodeResult inst, bool ignore_cond);
diff --git a/lldb/unittests/Instruction/CMakeLists.txt b/lldb/unittests/Instruction/CMakeLists.txt
index 10385377923ba..6a35b1c5b02d6 100644
--- a/lldb/unittests/Instruction/CMakeLists.txt
+++ b/lldb/unittests/Instruction/CMakeLists.txt
@@ -2,9 +2,11 @@ add_lldb_unittest(EmulatorTests
   ARM64/TestAArch64Emulator.cpp
   LoongArch/TestLoongArchEmulator.cpp
   RISCV/TestRISCVEmulator.cpp
+  RISCV/TestRiscvInstEmulation.cpp
 
   LINK_COMPONENTS
     Support
+    ${LLVM_TARGETS_TO_BUILD}
   LINK_LIBS
     lldbCore
     lldbSymbol
@@ -12,4 +14,7 @@ add_lldb_unittest(EmulatorTests
     lldbPluginInstructionARM64
     lldbPluginInstructionLoongArch
     lldbPluginInstructionRISCV
+    lldbPluginDisassemblerLLVMC
+    lldbPluginUnwindAssemblyInstEmulation
+    lldbPluginProcessUtility
   )
diff --git a/lldb/unittests/Instruction/RISCV/TestRiscvInstEmulation.cpp b/lldb/unittests/Instruction/RISCV/TestRiscvInstEmulation.cpp
new file mode 100644
index 0000000000000..7d9344df77465
--- /dev/null
+++ b/lldb/unittests/Instruction/RISCV/TestRiscvInstEmulation.cpp
@@ -0,0 +1,188 @@
+//===-- TestRiscvInstEmulation.cpp ----------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.h"
+
+#include "lldb/Core/AddressRange.h"
+#include "lldb/Symbol/UnwindPlan.h"
+#include "lldb/Utility/ArchSpec.h"
+
+#include "Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h"
+#include "Plugins/Instruction/RISCV/EmulateInstructionRISCV.h"
+#include "Plugins/Process/Utility/lldb-riscv-register-enums.h"
+#include "llvm/Support/TargetSelect.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+class TestRiscvInstEmulation : public testing::Test {
+public:
+  static void SetUpTestCase();
+  static void TearDownTestCase();
+
+protected:
+};
+
+void TestRiscvInstEmulation::SetUpTestCase() {
+  llvm::InitializeAllTargets();
+  llvm::InitializeAllAsmPrinters();
+  llvm::InitializeAllTargetMCs();
+  llvm::InitializeAllDisassemblers();
+  DisassemblerLLVMC::Initialize();
+  EmulateInstructionRISCV::Initialize();
+}
+
+void TestRiscvInstEmulation::TearDownTestCase() {
+  DisassemblerLLVMC::Terminate();
+  EmulateInstructionRISCV::Terminate();
+}
+
+TEST_F(TestRiscvInstEmulation, TestSimpleRiscvFunction) {
+  ArchSpec arch("riscv64-unknown-linux-gnu");
+  // Enable compressed instruction support (RVC extension)
+  arch.SetFlags(ArchSpec::eRISCV_rvc);
+  std::unique_ptr<UnwindAssemblyInstEmulation> engine(
+      static_cast<UnwindAssemblyInstEmulation *>(
+          UnwindAssemblyInstEmulation::CreateInstance(arch)));
+  ASSERT_NE(nullptr, engine);
+
+  // RISC-V function with compressed and uncompressed instructions
+  //   0x0000: 1141          addi    sp, sp, -0x10
+  //   0x0002: e406          sd      ra, 0x8(sp)
+  //   0x0004: e022          sd      s0, 0x0(sp)
+  //   0x0006: 0800          addi    s0, sp, 0x10
+  //   0x0008: 00000537      lui     a0, 0x0
+  //   0x000C: 00050513      mv      a0, a0
+  //   0x0010: 00000097      auipc   ra, 0x0
+  //   0x0014: 000080e7      jalr    ra <main+0x10>
+  //   0x0018: 4501          li      a0, 0x0
+  //   0x001A: ff040113      addi    sp, s0, -0x10
+  //   0x001E: 60a2          ld      ra, 0x8(sp)
+  //   0x0020: 6402          ld      s0, 0x0(sp)
+  //   0x0022: 0141          addi    sp, sp, 0x10
+  //   0x0024: 8082          ret
+  uint8_t data[] = {// 0x0000: 1141          addi sp, sp, -0x10
+                    0x41, 0x11,
+                    // 0x0002: e406          sd ra, 0x8(sp)
+                    0x06, 0xE4,
+                    // 0x0004: e022          sd s0, 0x0(sp)
+                    0x22, 0xE0,
+                    // 0x0006: 0800          addi s0, sp, 0x10
+                    0x00, 0x08,
+                    // 0x0008: 00000537      lui a0, 0x0
+                    0x37, 0x05, 0x00, 0x00,
+                    // 0x000C: 00050513      mv a0, a0
+                    0x13, 0x05, 0x05, 0x00,
+                    // 0x0010: 00000097      auipc ra, 0x0
+                    0x97, 0x00, 0x00, 0x00,
+                    // 0x0014: 000080e7      jalr ra <main+0x10>
+                    0xE7, 0x80, 0x00, 0x00,
+                    // 0x0018: 4501          li a0, 0x0
+                    0x01, 0x45,
+                    // 0x001A: ff040113      addi sp, s0, -0x10
+                    0x13, 0x01, 0x04, 0xFF,
+                    // 0x001E: 60a2          ld ra, 0x8(sp)
+                    0xA2, 0x60,
+                    // 0x0020: 6402          ld s0, 0x0(sp)
+                    0x02, 0x64,
+                    // 0x0022: 0141          addi sp, sp, 0x10
+                    0x41, 0x01,
+                    // 0x0024: 8082          ret
+                    0x82, 0x80};
+
+  // Expected UnwindPlan (prologue only - emulation stops after frame setup):
+  // row[0]:    0:  CFA=sp+0   => fp= <same>        ra= <same>
+  // row[1]:    2:  CFA=sp+16  => fp= <same>        ra= <same>      (after stack
+  // allocation) row[2]:    4:  CFA=sp+16  => fp= <same>        ra=[CFA-8]
+  // (after saving ra) row[3]:    6:  CFA=sp+16  => fp=[CFA-16]       ra=[CFA-8]
+  // (after saving s0/fp) row[4]:    8:  CFA=s0+0   => fp=[CFA-16] ra=[CFA-8]
+  // (after setting frame pointer: s0=sp+16)
+
+  const UnwindPlan::Row *row;
+  AddressRange sample_range;
+  UnwindPlan unwind_plan(eRegisterKindLLDB);
+  UnwindPlan::Row::AbstractRegisterLocation regloc;
+  sample_range = AddressRange(0x1000, sizeof(data));
+
+  EXPECT_TRUE(engine->GetNonCallSiteUnwindPlanFromAssembly(
+      sample_range, data, sizeof(data), unwind_plan));
+
+  // CFA=sp+0 => fp=<same> ra=<same>
+  row = unwind_plan.GetRowForFunctionOffset(0);
+  EXPECT_EQ(0, row->GetOffset());
+  EXPECT_TRUE(row->GetCFAValue().GetRegisterNumber() == gpr_sp_riscv);
+  EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset() == true);
+  EXPECT_EQ(0, row->GetCFAValue().GetOffset());
+
+  EXPECT_TRUE(row->GetRegisterInfo(gpr_fp_riscv, regloc));
+  EXPECT_TRUE(regloc.IsSame());
+
+  EXPECT_TRUE(row->GetRegisterInfo(gpr_ra_riscv, regloc));
+  EXPECT_TRUE(regloc.IsSame());
+
+  // CFA=sp+16 => fp=<same> ra=<same>
+  row = unwind_plan.GetRowForFunctionOffset(2);
+  EXPECT_EQ(2, row->GetOffset());
+  EXPECT_TRUE(row->GetCFAValue().GetRegisterNumber() == gpr_sp_riscv);
+  EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset() == true);
+  EXPECT_EQ(16, row->GetCFAValue().GetOffset());
+
+  EXPECT_TRUE(row->GetRegisterInfo(gpr_fp_riscv, regloc));
+  EXPECT_TRUE(regloc.IsSame());
+
+  EXPECT_TRUE(row->GetRegisterInfo(gpr_ra_riscv, regloc));
+  EXPECT_TRUE(regloc.IsSame());
+
+  // CFA=sp+16 => fp=<same> ra=[CFA-8]
+  row = unwind_plan.GetRowForFunctionOffset(4);
+  EXPECT_EQ(4, row->GetOffset());
+  EXPECT_TRUE(row->GetCFAValue().GetRegisterNumber() == gpr_sp_riscv);
+  EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset() == true);
+  EXPECT_EQ(16, row->GetCFAValue().GetOffset());
+
+  EXPECT_TRUE(row->GetRegisterInfo(gpr_fp_riscv, regloc));
+  EXPECT_TRUE(regloc.IsSame());
+
+  EXPECT_TRUE(row->GetRegisterInfo(gpr_ra_riscv, regloc));
+  EXPECT_TRUE(regloc.IsAtCFAPlusOffset());
+  EXPECT_EQ(-8, regloc.GetOffset());
+
+  // CFA=sp+16 => fp=[CFA-16] ra=[CFA-8]
+  row = unwind_plan.GetRowForFunctionOffset(6);
+  EXPECT_EQ(6, row->GetOffset());
+  EXPECT_TRUE(row->GetCFAValue().GetRegisterNumber() == gpr_sp_riscv);
+  EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset() == true);
+  EXPECT_EQ(16, row->GetCFAValue().GetOffset());
+
+  EXPECT_TRUE(row->GetRegisterInfo(gpr_fp_riscv, regloc));
+  EXPECT_TRUE(regloc.IsAtCFAPlusOffset());
+  EXPECT_EQ(-16, regloc.GetOffset());
+
+  EXPECT_TRUE(row->GetRegisterInfo(gpr_ra_riscv, regloc));
+  EXPECT_TRUE(regloc.IsAtCFAPlusOffset());
+  EXPECT_EQ(-8, regloc.GetOffset());
+
+  // CFA=s0+0 => fp=[CFA-16] ra=[CFA-8]
+  // s0 = sp + 16, so switching CFA to s0 does not change the effective
+  // locations.
+  row = unwind_plan.GetRowForFunctionOffset(8);
+  EXPECT_EQ(8, row->GetOffset());
+  EXPECT_TRUE(row->GetCFAValue().GetRegisterNumber() == gpr_fp_riscv);
+  EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset() == true);
+  EXPECT_EQ(0, row->GetCFAValue().GetOffset());
+
+  EXPECT_TRUE(row->GetRegisterInfo(gpr_fp_riscv, regloc));
+  EXPECT_TRUE(regloc.IsAtCFAPlusOffset());
+  EXPECT_EQ(-16, regloc.GetOffset());
+
+  EXPECT_TRUE(row->GetRegisterInfo(gpr_ra_riscv, regloc));
+  EXPECT_TRUE(regloc.IsAtCFAPlusOffset());
+  EXPECT_EQ(-8, regloc.GetOffset());
+}
\ No newline at end of file

return false;

// Set context type based on whether this is a stack-based load
if (inst.rs1.rs == RISCV_GPR_SP) // x2 is the stack pointer in RISC-V
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably don't need this comment now that we are using RISCV_GPR_SP?

return false;

// Set context type based on whether this is a stack-based store
if (inst.rs1.rs == RISCV_GPR_SP) // x2 is the stack pointer in RISC-V
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably don't need this comment now that we are using RISCV_GPR_SP?

int64_t result = rs1 + int64_t(SignExt(inst.imm));
// Check if this is a stack pointer adjustment
if (inst.rd.rd == RISCV_GPR_SP &&
inst.rs1.rs == RISCV_GPR_SP) { // rd=sp, rs1=sp
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably don't need this comment now that we are using RISCV_GPR_SP?

Comment on lines 821 to 822
if (inst.rd.rd == RISCV_GPR_FP &&
inst.rs1.rs == RISCV_GPR_SP) { // rd=fp, rs1=sp
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably don't need this comment now that we are using RISCV_GPR_SP and RISCV_GPR_FP?

@satyajanga
Copy link
Contributor

@jasonmolenda Can you please review this, this is a followup of #147434 to properly address the emulation and unwinding.

@clayborg
Copy link
Collaborator

I added some frequent editors of this file to the reviewers in case they have any comments.

Copy link
Member

@JDevlieghere JDevlieghere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General feedback: Comment should end with periods.

We have a little RISC-V board that runs Ubuntu [1]. It would be interesting to see if this improves the test results, but that doesn't block this PR.

[1] #55383 (comment)

@satyajanga
Copy link
Contributor

looks good to me.

@satyajanga satyajanga merged commit 1c95d80 into llvm:main Sep 18, 2025
9 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Sep 18, 2025

LLVM Buildbot has detected a new failure on builder lldb-remote-linux-ubuntu running on as-builder-9 while building lldb at step 15 "test-check-lldb-unit".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/195/builds/14788

Here is the relevant piece of the build log for the reference
Step 15 (test-check-lldb-unit) failure: Test just built components: check-lldb-unit completed (failure)
******************** TEST 'lldb-unit :: Instruction/./EmulatorTests/62/63' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/tools/lldb/unittests/Instruction/./EmulatorTests-lldb-unit-3928114-62-63.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=63 GTEST_SHARD_INDEX=62 /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/tools/lldb/unittests/Instruction/./EmulatorTests
--

Script:
--
/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/tools/lldb/unittests/Instruction/./EmulatorTests --gtest_filter=TestRiscvInstEmulation.TestSimpleRiscvFunction
--
/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/unittests/Instruction/RISCV/TestRiscvInstEmulation.cpp:114: Failure
Value of: engine->GetNonCallSiteUnwindPlanFromAssembly( sample_range, data, sizeof(data), unwind_plan)
  Actual: false
Expected: true

/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/unittests/Instruction/RISCV/TestRiscvInstEmulation.cpp:132: Failure
Expected equality of these values:
  2
  row->GetOffset()
    Which is: 0

/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/unittests/Instruction/RISCV/TestRiscvInstEmulation.cpp:135: Failure
Expected equality of these values:
  16
  row->GetCFAValue().GetOffset()
    Which is: 0

/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/unittests/Instruction/RISCV/TestRiscvInstEmulation.cpp:145: Failure
Expected equality of these values:
  4
  row->GetOffset()
    Which is: 0

/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/unittests/Instruction/RISCV/TestRiscvInstEmulation.cpp:148: Failure
Expected equality of these values:
  16
  row->GetCFAValue().GetOffset()
    Which is: 0

/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/unittests/Instruction/RISCV/TestRiscvInstEmulation.cpp:154: Failure
Value of: regloc.IsAtCFAPlusOffset()
  Actual: false
Expected: true

/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/unittests/Instruction/RISCV/TestRiscvInstEmulation.cpp:155: Failure
Expected equality of these values:
  -8
  regloc.GetOffset()
    Which is: 0

...

@slydiman
Copy link
Contributor

slydiman commented Sep 19, 2025

LLVM Buildbot has detected a new failure on builder lldb-remote-linux-ubuntu
https://lab.llvm.org/buildbot/#/builders/195/builds/14788

The buildbot lldb-remote-linux-win is broken too
https://lab.llvm.org/buildbot/#/builders/197/builds/9007

Please look at this and fix ASAP.

@felipepiovezan
Copy link
Contributor

LLVM Buildbot has detected a new failure on builder lldb-remote-linux-ubuntu
https://lab.llvm.org/buildbot/#/builders/195/builds/14788

The buildbot lldb-remote-linux-win is broken too https://lab.llvm.org/buildbot/#/builders/197/builds/9007

Please look at this and fix ASAP.

@barsolo2000 the macos bots are also broken: https://ci.swift.org/view/all/job/llvm.org/view/LLDB/job/lldb-cmake/15260/consoleFull

06:46:01 lldb-unit :: Instruction/./EmulatorTests/TestRiscvInstEmulation/TestSimpleRiscvFunction

It's the same test as the other bots mentioned above.

It's been 18 hours since the first reported failure, please revert if this won't be fixed in a timely fashion.

DavidSpickett added a commit that referenced this pull request Sep 19, 2025
DavidSpickett added a commit that referenced this pull request Sep 19, 2025
Reverts #158161

Due to reported failures on remote Linux and Swift buildbots.
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Sep 19, 2025
Reverts llvm/llvm-project#158161

Due to reported failures on remote Linux and Swift buildbots.
satyajanga pushed a commit that referenced this pull request Sep 22, 2025
Initial PR was reverted due failing test since the buildbots don't
include the RISCV backend.

---------

Co-authored-by: Bar Soloveychik <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants